Support for sparse Eigen matrices in Hpolytope class #327
Conversation
lucaperju
commented
Jul 22, 2024
lucaperju
commented
- Add possibility to generate Hpolytopes with sparse A matrix
- Add possibility to sample from them using GABW and ABW, with sparse covariance matrix
include/convex_bodies/hpolytope.h
Outdated
| //using RowMatrixXd = Eigen::Matrix<NT, Eigen::Dynamic, Eigen::Dynamic, Eigen::RowMajor>; | ||
| //typedef RowMatrixXd MT; |
include/convex_bodies/hpolytope.h
Outdated
| @@ -509,7 +524,7 @@ class HPolytope { | |||
| if(params.hit_ball) { | |||
| Av.noalias() += (-2.0 * inner_prev) * (Ar / params.ball_inner_norm); | |||
| } else { | |||
| Av.noalias() += (-2.0 * inner_prev) * AA.col(params.facet_prev); | |||
| Av.noalias() += (DenseMT)((-2.0 * inner_prev) * AA.col(params.facet_prev)); | |||
There was a problem hiding this comment.
Input matrix AA should always be dense. Then, (DenseMT) is not needed here.
| @@ -630,12 +645,12 @@ class HPolytope { | |||
|
|
|||
| int m = num_of_hyperplanes(); | |||
|
|
|||
| lamdas.noalias() += A.col(rand_coord_prev) | |||
| * (r_prev[rand_coord_prev] - r[rand_coord_prev]); | |||
| lamdas.noalias() += (DenseMT)(A.col(rand_coord_prev) | |||
There was a problem hiding this comment.
Would lamdas.noalias() += (VT)(A.col(rand_coord_prev) ... work here as well?
| @@ -100,6 +112,7 @@ class HPolytope { | |||
| void set_InnerBall(std::pair<Point,NT> const& innerball) //const | |||
| { | |||
| _inner_ball = innerball; | |||
| has_ball = true; | |||
| } | |||
|
|
|||
| void set_interior_point(Point const& r) | |||
There was a problem hiding this comment.
Can you add a TODO here to fix this function?
Because when we change the center of the inner ball we also need to change its radius.
Also the has_ball should be set to false here, right?
There was a problem hiding this comment.
I decided to set the has_ball to true here since I assumed that whenever this function might be used, it would be given a valid innerball, but tbh I haven't seen anywhere in the code where this function is called anyways.
edit: oh, if you're refering the the set_interior_point function, then yeah, has_ball should likely be set to false, but I don't really know how we could fix this function further than that
There was a problem hiding this comment.
Fisrt, we need to check that the point is indeed inside the polytope and only then set it as center of the inner ball. Then, we need to compute the distances of the center from all the facets and take the minimum. The latter distance is the radius of the maximum inscribed ball centered at the given point.
The distance between a point p and the i-th facet is given by: (b_i - a_i^Tp) / ||a_i||.
Check also the function get_dists() below; it returns the distance from the origin to each facet.
|
Thanks @lucaperju! That's a very useful PR! |
